-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #235 allocatable classes #236
Fix #235 allocatable classes #236
Conversation
@jameskermode this PR is ready for a full review and merge. @Rykath, @Xyhlon - please also have a look and comment if you see something odd. The code now
The solution involves adding a The change is significant, as the wrapper type is introduced also in cases that were previously handled with |
@jameskermode could you already take a look at this PR? In the meantime we could finish also #233 to fix #41 on top of this. PS: Tests are passing, but the "Build wheel" jobs are causing troubles and I don't understand why. |
Thanks very much for this contribution. As you say it's a major chance so will need testing with some downstream codes wrapped with f90wrap to ensure there are no regressions not captured in the unit tests. I should have time to do that next week. Do I understand right that this PR should be merged before #233? |
Sure! For now the main work is done and there is no hurry. You are right, #233 should be merged after this one. We will also be testing on our fork on https://github.com/itpplasma with some of our codes in the meantime. With #233, f90wrap will cover all the remaining requirements for interfacing modern code from a compiled language in Python with minimal effort (C++/pybind11 people already envy us now). |
... or if you have time for a short videocall to discuss some points directly, just drop me an e-mail then. |
f1d94a0
to
d95e13d
Compare
d95e13d
to
db919ac
Compare
99c2be1
to
3a174a5
Compare
3a174a5
to
9ad5387
Compare
I will go ahead and merge this and #233 but hold off from doing a release until testing with some more downstream codes. Thanks again for all the contributions |
Fixes #235 . This PR sits on top of #230, #231, #232, #237, #238 and #239.